Skip to content

Conversation

bakpaul
Copy link
Contributor

@bakpaul bakpaul commented Aug 22, 2025

This method's goal is to reduce as much as possible thee data serialization

self.assertEqual(root.FirstNode.objects()[0].type.value, "RequiredPlugin")
self.assertEqual(root.FirstNode.objects()[0].name.value, "FirstNode")

pass
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pass

self.assertEqual(root.objects()[0].type.value, "RequiredPlugin")
self.assertEqual(root.objects()[0].name.value, "FirstNode")

pass
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pass

root.add(MyNode)
self.assertEqual(root.children()[0].name.value, "MyNode")

pass
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pass

}
}

py::object add( Node* self, const std::string& type, const py::kwargs& kwargs)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to add a second signature so we can pass real type instead of string.

py::object add( Node* self, py::type type, const py::kwargs& kwargs)
{
   if(issubchild(type, BaseObject)){
         ... 
   }else if(issubchild(type, Node)){
           ....
   }
}

But this could be in a second PR.

name = py::str(kwargs["name"]);
if (sofapython3::isProtectedKeyword(name))
throw py::value_error("Cannot call addObject with name " + name + ": Protected keyword");
}
Copy link
Contributor

@damienmarchal damienmarchal Sep 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we consider to add a name generation mechanism (with +1) to avoid having name duplicated.
Of course this have a non linear performance hit in case of gigantic scene... but I would be in favor of having a robust normal add() and a second one considered "fast mode" for expert users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants